Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENH] added PPG as an accepted channel type for EEG, MEG and iEEG #570

Merged
merged 1 commit into from
Aug 17, 2020

Conversation

robertoostenveld
Copy link
Collaborator

@robertoostenveld robertoostenveld commented Aug 12, 2020

See #561 and bids-standard/bids-examples#197 (comment)

This does not close ... #561 since I think that the ACCEL requires some more thought and discussion with other stakeholders.

…r EEG, whcih also makes it acceptable for MEG and iEEG. See bids-standard#561
Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @robertoostenveld would we have to add this entry to the tables in the MEG and iEEG sections as well?

I think the current understanding among the ephys modalities is, that these types are shared anyhow.

@rwblair could you please update this in the validator and revert the changes in the bids-examples PR where you changed PPG to PD?

@robertoostenveld
Copy link
Collaborator Author

robertoostenveld commented Aug 12, 2020

would we have to add this entry to the tables in the MEG and iEEG sections as well?

the way I read the specification is that each of the MEG, EEG and iEEG chapters lists the channel types that are common for that specific modality, but also allows for the specification of channel types from the others. I.e. the lists are shared between MEG, EEG and iEEG, and a complete list would be the union of the three.

I think better (more maintainable, and more clear for people who read it) would be to have the channel types in an appendix. That would also be good for upcoming modalities (NIRS, motion). But that is considerably more work, and not in scope of this small PR.

Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants